Skip to content

Conversation

@mikeingold
Copy link
Collaborator

@mikeingold mikeingold commented Jun 24, 2025

Changes

  • Adds integral methods for Meshes.PolyArea and subtypes of Meshes.Domain with explicit tests for
    • CartesianGrid
    • PolyArea
    • RegularGrid
    • SimpleMesh
  • Generalizes the alias functions (e.g. lineintegral) to also accept Domains.
  • Update docstrings to improve clarity and consistency.

Closes:

@mikeingold mikeingold self-assigned this Jun 24, 2025
@mikeingold mikeingold added the enhancement New feature or request label Jun 24, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Jun 24, 2025

Benchmark Results

main 4e7c1cd... main / 4e7c1cd...
Differentials/Differential 0.203 ± 0.001 μs 0.205 ± 0.002 μs 0.99 ± 0.011
Differentials/Jacobian 0.17 ± 0.0011 μs 0.169 ± 0.001 μs 1.01 ± 0.0088
Integrals/Segment/Scalar GaussKronrod 0.552 ± 0.0083 μs 0.585 ± 0.0092 μs 0.944 ± 0.02
Integrals/Segment/Scalar GaussLegendre 1.61 ± 0.008 μs 1.68 ± 0.0079 μs 0.962 ± 0.0066
Integrals/Segment/Scalar HAdaptiveCubature 0.714 ± 0.021 μs 0.727 ± 0.018 μs 0.983 ± 0.038
Integrals/Segment/Vector GaussKronrod 2.78 ± 0.11 μs 2.8 ± 0.11 μs 0.994 ± 0.055
Integrals/Segment/Vector GaussLegendre 18.4 ± 1.2 μs 18 ± 1 μs 1.03 ± 0.09
Integrals/Segment/Vector HAdaptiveCubature 3.8 ± 0.15 μs 3.63 ± 0.19 μs 1.05 ± 0.069
Integrals/Sphere/Scalar GaussKronrod 0.0693 ± 0.0019 ms 0.069 ± 0.0019 ms 1 ± 0.039
Integrals/Sphere/Scalar GaussLegendre 1.87 ± 0.01 ms 1.87 ± 0.0054 ms 0.999 ± 0.0063
Integrals/Sphere/Scalar HAdaptiveCubature 0.0476 ± 0.00011 ms 0.0477 ± 0.00012 ms 0.999 ± 0.0034
Integrals/Sphere/Vector GaussKronrod 0.104 ± 0.002 ms 0.105 ± 0.0041 ms 0.997 ± 0.044
Integrals/Sphere/Vector GaussLegendre 3.36 ± 0.1 ms 3.32 ± 0.15 ms 1.01 ± 0.056
Integrals/Sphere/Vector HAdaptiveCubature 0.102 ± 0.0023 ms 0.1 ± 0.0025 ms 1.01 ± 0.034
Rules/GaussLegendre 22.2 ± 1.6 μs 22.7 ± 0.95 μs 0.978 ± 0.081
Specializations/Scalar GaussLegendre/BezierCurve 0.253 ± 0.0071 ms 0.253 ± 0.0072 ms 1 ± 0.04
Specializations/Scalar GaussLegendre/Line 6.94 ± 0.068 μs 7.23 ± 0.13 μs 0.959 ± 0.02
Specializations/Scalar GaussLegendre/Plane 0.745 ± 0.0024 ms 0.746 ± 0.0021 ms 0.999 ± 0.0043
Specializations/Scalar GaussLegendre/Ray 5.85 ± 0.065 μs 5.9 ± 0.058 μs 0.99 ± 0.015
Specializations/Scalar GaussLegendre/Rope 0.0515 ± 0.00023 ms 0.0527 ± 0.0002 ms 0.979 ± 0.0057
Specializations/Scalar GaussLegendre/Tetrahedron 0.157 ± 0.0024 s 0.158 ± 0.0033 s 0.995 ± 0.026
Specializations/Scalar GaussLegendre/Triangle 0.602 ± 0.0075 ms 0.609 ± 0.0075 ms 0.989 ± 0.017
time_to_load 1.43 ± 0.013 s 1.45 ± 0.014 s 0.987 ± 0.013

Benchmark Plots

A plot of the benchmark results have been uploaded as an artifact to the workflow run for this PR.
Go to "Actions"->"Benchmark a pull request"->[the most recent run]->"Artifacts" (at the bottom).

@mikeingold
Copy link
Collaborator Author

Interesting. The remaining errors with the SimpleMesh tests are invalid barycentric coords errors from integrating Triangles.

@mikeingold
Copy link
Collaborator Author

mikeingold commented Jun 28, 2025

Still debugging. One of the remaining issues (UndefVarError: geometry not defined) has a stack trace that appeared at first to be throwiing directly from combinations.jl:138's use of geometry, but I've done some debugging and it appears to be coming from some other function further along the stack.

Edit: got it. There was a holdover in the default rule selection for the integral(::Domain) method.

@mikeingold
Copy link
Collaborator Author

mikeingold commented Jul 5, 2025

I've run some tests locally to figure out why the Meshes.CartesianGrid test set takes so long, and I've run across some odd results.

  • The test set initially took about 80 minutes to run on my computer.
  • I ran some tests in a separate REPL and couldn't find any obvious slow integration. Then I tried adding some Logging.@debug lines to give me a quick trace on where the tests are spending all of their time, and suddenly it would repeatably run in about 15 seconds.
  • I manually reverted my local repo state to the current commit to make sure there wasn't some unnoticed change and then I rebooted VS Code. Re-running the test set, the first run took about 12 minutes, but then successive runs took 10-15 seconds.

@mikeingold
Copy link
Collaborator Author

Based on a little more debugging, even in the longer CartesianGrid runs the actual runtests function only requires 10-15 seconds, so I suspect that TestItems is also measuring compilation time and that must be dominating the first runtime in this case. Odd that it only seems to be an issue here and not also for RegularGrid. Maybe some kind of type instability/inference issue?

@mikeingold
Copy link
Collaborator Author

I had a vague suspicion that turned out to be true. The really extreme CI times were nearly 100% compilation time. The output of elements(discretize(geometry)) is a generator, so I wondered if additionally collecting this array before integration would simplify things for the compiler at the cost of a single small additional allocation. Based on the new CI times this seems to have worked.

mikeingold and others added 3 commits July 17, 2025 22:55
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@mikeingold
Copy link
Collaborator Author

@juliohm Should Meshes.GeometryOrDomain be exported? This PR currently triggers a CI fail (ExplicitImports.jl) over importing it as a non-public/exported reference. I can always just define it locally if it wasn't intended for stable external use.

@juliohm
Copy link
Member

juliohm commented Jul 19, 2025 via email

@codecov
Copy link

codecov bot commented Jul 19, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (5c7a06e) to head (4e7c1cd).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #182   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           18        19    +1     
  Lines          181       190    +9     
=========================================
+ Hits           181       190    +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mikeingold mikeingold changed the title Add support for integrating over <:Meshes.Domain types Add support for integrating over PolyArea and Domain types Jul 19, 2025
@mikeingold mikeingold marked this pull request as ready for review July 20, 2025 18:19
@mikeingold mikeingold requested a review from JoshuaLampert July 20, 2025 18:19
Copy link
Member

@JoshuaLampert JoshuaLampert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really nice! I only have some minor comments.

@mikeingold
Copy link
Collaborator Author

I accepted the suggestions but had to reformat the _default_rule definition to avoid exceeding our max line length formatting rule.

Copy link
Member

@JoshuaLampert JoshuaLampert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@mikeingold mikeingold merged commit 8bd66b4 into main Jul 21, 2025
12 checks passed
@mikeingold mikeingold deleted the remesh branch July 21, 2025 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Integral on PolyArea not yet implemented Integral on SimpleMesh not yet implemented

4 participants